-
Notifications
You must be signed in to change notification settings - Fork 383
Refactor drag and drop #2667
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor drag and drop #2667
Conversation
| // Note: the calculation differs with IsAttacking because | ||
| // frames are intended to behave differently with normal sprite-sheets. | ||
| else if (AttackTimer - (CalculateAttackTime() / 2) > Timing.Global.Milliseconds) | ||
| else if (AttackTimer - (CalculateAttackTime() / 2) > Timing.Global.Milliseconds && !DragAndDrop.IsDragging) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is drag and drop code in Entity.NormalSpriteAnimationFrame???
| if (DragAndDrop.IsDragging) | ||
| { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not belong here.
Presumably this is to stop the left click from triggering an attack when dragging and dropping, but this should be processed in the controls instead (i.e. if dragging and dropping, the controls binding should not return "active" for left mouse button). There shouldn't be controls logic in this method.
| MouseInputEnabled = true; | ||
|
|
||
| _iconImage = new ImagePanel(this, "Icon") | ||
| IconImage = new Draggable(this, "Icon") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename to Icon and change the name to nameof(Icon)
| _descWindow?.Dispose(); | ||
| _descWindow = default; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_descriptionWindow
| { | ||
| // ReSharper disable once SwitchStatementMissingSomeEnumCasesNoDefault | ||
| switch (arguments.MouseButton) | ||
| if (Globals.InBag) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs to check left click
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also I don't think we even need the Globals.InBag check... the window shouldn't be open (meaning BagItem isn't visible) if it's not true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(So yeah, replace Globals.InBag with arguments.MouseButton is MouseButton.Left)
| // If we've reached the top of the tree, we can't drop here, so cancel drop | ||
| if (targetNode == null) | ||
| { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unnecessary, immediately after this if-statement it would hit the while condition and return false anyway
| public override bool DragAndDrop_HandleDrop(Package package, int x, int y) | ||
| { | ||
| if (Globals.InBag) | ||
| var targetNode = Interface.FindComponentUnderCursor(NodeFilter.None); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need NodeFilter.None, the parameter is optional and None is the default.
| } | ||
|
|
||
| if (!IsDragging) | ||
| var itemTexture = Globals.ContentManager?.GetTexture(Framework.Content.TextureType.Item, descriptor.Icon); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Globals.ContentManager? -> GameContentManager.Current (no ?)
|
|
||
| public override bool DragAndDrop_HandleDrop(Package package, int x, int y) | ||
| { | ||
| if (Globals.Me?.Inventory is not { } inventory) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cache Globals.Me before everything and return false if null
| return true; | ||
| } | ||
|
|
||
| var targetNode = Interface.FindComponentUnderCursor(NodeFilter.None); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove NodeFilter.None
| // If we've reached the top of the tree, we can't drop here, so return false | ||
| if (targetNode == null) | ||
| { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove, unnecessary
| } | ||
|
|
||
| if (!IsDragging) | ||
| var itemTexture = Globals.ContentManager?.GetTexture(Framework.Content.TextureType.Item, descriptor.Icon); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GameContentManager.Current
| } | ||
|
|
||
| _dragIcon.Dispose(); | ||
| if (_descWindow != null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_descriptionWindow
| if (!isInGuild || (player.Rank != 0 && rank?.Permissions.BankDeposit == false)) | ||
| { | ||
| ChatboxMsg.AddMessage( | ||
| new ChatboxMsg( | ||
| Strings.Guilds.NotAllowedSwap.ToString(player.Guild), | ||
| CustomColors.Alerts.Error, | ||
| ChatMessageType.Bank | ||
| ) | ||
| ); | ||
|
|
||
| return false; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this breaking regular bank if you're not a guildmaster?
| if (IsMouseKey && Key.TryGetMouseButton(out var mouseButton)) | ||
| { | ||
| return gameInput.IsMouseButtonDown(mouseButton); | ||
| return gameInput.IsMouseButtonDown(mouseButton) && !DragAndDrop.IsDragging; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also needs to be in WasDown to avoid unexpected mouse "release" behavior.
| public bool IsDragging => DragAndDrop.CurrentPackage?.DrawControl == this; | ||
|
|
||
| public int Y | ||
| public override bool DragAndDrop_Draggable() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where's the TODO? xD
redoing the drag and drop to use the native gwen and not a bunch of calculations checking intersections of rectangles
What's going on here
Gwen's drag and drop system works based on "sending packages" and "receiving packages", with some situations where you can benefit from such as starting the drag, accepting the packet, when it drops (these are not events, unfortunately)
Anyone who wants to create their draggable image component just needs to inherit from Draggable, so I made all the IconImages of the SlotsItens as Draggables, which already comes with a "default configuration" of what a draggable component should be
Furthermore, in the Gwen Base component, I made it so that all components are able to accept any package, but no real drop will happen if the
public override bool DragAndDrop_HandleDrop(Package package, int x, int y)is not implemented in child, which is responsible for returning a response to the component being dragged whether the drop was successful or not.
The basic implementation I did for all items was a loop, starting from the current component to the root, checking what the component is, because if it is acceptable something will be done and true will be returned, if not, when returning false, we cancel the drop.
For example, when dragging an IconImage from a BankItem to an empty inventory slot, this component is an InventoryItem, so it will be removed from the bank, but if the slot is occupied, then the target component was a Draggable, it could be a label too, and so on, which is why the previous Parents are checked, Parent will be InventoryItem and it will be removed from the bank.
I kept the default behavior above all, hiding labels and such when dragging, and I also added behavior to not close windows or hide UI when dragging which caused this bug (latest clean)
1741895493_Intersect_Client.mp4
and finally I added a new function which is to drag the item into the shop window and it will be sold
full video of everything working
1741892635_Intersect_Client.mp4